Skip to content

Conversation

@roomote
Copy link
Collaborator

@roomote roomote commented Jun 30, 2025

Summary

This PR implements the 'Add & Run' button feature requested in issue #5290. The feature adds a third button to the command approval dialog that allows users to both whitelist a command and execute it in a single action, streamlining the workflow for users with auto-approve mode enabled.

Changes Made

Backend Implementation

  • Modified ClineAskResponse type in src/shared/WebviewMessage.ts to include 'addAndRunButtonClicked' option
  • Updated presentAssistantMessage.ts to handle both 'yesButtonClicked' and 'addAndRunButtonClicked' as approval responses in the askApproval function
  • Enhanced executeCommandTool.ts to properly detect when 'Add & Run' was selected and add the command to the whitelist using the correct API methods
  • Added tertiaryButtonClick to ExtensionMessage type in src/shared/ExtensionMessage.ts

UI Implementation

  • Modified ChatView.tsx to support three buttons with conditional rendering
  • Added tertiaryButtonText state variable to support the third button
  • Updated command case to set button texts: primary='Run Command', secondary='Add & Run', tertiary='Reject'
  • Implemented three-button layout: 'Run' and 'Add & Run' buttons on top row, 'Reject' on bottom row
  • Added message handling for tertiary button interactions

Technical Details

  • Command Approval Flow: Uses askApproval function that returns boolean values, with actual response captured via cline.ask() method
  • Message Passing: Enhanced extension and webview communication via ExtensionMessage and WebviewMessage types
  • Command Validation: Integrates with existing prefix matching in validateCommand function with allowedCommands array
  • Settings Management: Uses ClineProvider.setValue() method to persist whitelist changes
  • UI Architecture: Maintains existing React-based webview UI with VSCode toolkit components

Testing

  • ✅ All existing tests pass
  • ✅ TypeScript compilation successful
  • ✅ ESLint checks pass
  • ✅ Three-button layout renders correctly
  • ✅ Message passing system works for tertiary button
  • ✅ Command whitelisting functionality integrated

User Experience

The new workflow allows users to:

  1. See a command approval dialog with three clear options
  2. Click 'Add & Run' to both whitelist the command and execute it immediately
  3. Future executions of the same command will be auto-approved
  4. Maintain existing 'Run Command' and 'Reject' functionality

This eliminates the need to manually navigate to settings after approving frequently used commands, significantly improving the user experience for users with auto-approve mode enabled.

Fixes #5290


Important

Adds 'Add & Run' button to command approval UI, allowing users to whitelist and execute commands in one step, with backend and UI updates.

  • Behavior:
    • Adds 'Add & Run' button to command approval UI, allowing users to whitelist and execute commands in one step.
    • Updates presentAssistantMessage.ts to handle 'addAndRunButtonClicked' response.
    • Enhances executeCommandTool.ts to detect 'Add & Run' selection and add command to whitelist.
  • UI Changes:
    • Modifies ChatView.tsx to support three-button layout: 'Run', 'Add & Run', 'Reject'.
    • Adds tertiaryButtonText state for third button.
  • Types and Messages:
    • Updates ClineAskResponse in WebviewMessage.ts to include 'addAndRunButtonClicked'.
    • Adds tertiaryButtonClick to ExtensionMessage in ExtensionMessage.ts.
  • Misc:
    • Adds extractCommandPattern() to command-validation.ts for command whitelisting.
    • Updates i18n files for new button text.

This description was created by Ellipsis for 82c3e92. You can customize this summary. It will automatically update as commits are pushed.

- Add third button option to command approval dialog
- Implement streamlined whitelisting workflow that adds command to whitelist and executes it
- Update UI to show three buttons: 'Run Command', 'Add & Run', and 'Reject'
- Add backend logic to detect 'Add & Run' selection and update allowedCommands setting
- Enhance message passing system to support tertiary button interactions

Fixes #5290
@roomote roomote requested review from cte, jr and mrubens as code owners June 30, 2025 20:18
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. UI/UX UI/UX related or focused labels Jun 30, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jun 30, 2025

No security or compliance issues detected. Reviewed everything up to 82c3e92.

Security Overview
  • 🔎 Scanned files: 46 changed file(s)
Detected Code Changes

The diff is too large to display a summary of code changes.

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 30, 2025
Copy link
Collaborator

@hannesrudolph hannesrudolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing the 'Add & Run' feature! The UI implementation looks good, but there's a critical issue with how commands are being whitelisted that needs to be addressed.

Critical Issue

The current implementation adds the entire command with arguments to the whitelist (e.g., "gh pr checkout 1234"), but this defeats the purpose of intelligent whitelisting. Each variation of the same command would require separate approval:

  • gh pr checkout 1234
  • gh pr checkout 5678
  • npm install express
  • npm install react

Instead, we should be whitelisting command patterns that can be validated using prefix matching, aligning with how validateCommand already works.

Suggested Approach

Consider extracting the base command pattern before whitelisting. Here's a rough idea:

function extractBaseCommand(fullCommand: string): string {
  // Parse command to extract the base pattern
  // e.g., "gh pr checkout 1234" -> "gh pr checkout"
  // e.g., "npm install express" -> "npm install"
  const parts = fullCommand.trim().split(/\s+/);
  
  // This is simplified - you'd want more sophisticated parsing
  // to handle different command structures
  if (parts[0] === 'gh' && parts[1] === 'pr') {
    return parts.slice(0, 3).join(' ');
  }
  // ... more patterns
  
  return parts.slice(0, 2).join(' ');
}

Other Issues

  1. Code duplication: The approval logic is duplicated between executeCommandTool.ts and presentAssistantMessage.ts
  2. Error handling: Missing null checks when dereferencing the provider
  3. i18n: User-facing messages need internationalization
  4. Type safety: Consider stricter typing for the response handling

The feature itself is valuable and the three-button UI is well-implemented. With these changes, particularly the command pattern extraction, this will be a great addition to improve the user experience!


// Add command to whitelist if not already present
if (!currentCommands.includes(command)) {
const newCommands = [...currentCommands, command]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation whitelists the entire command including arguments (e.g., "gh pr checkout 1234"), but shouldn't we be whitelisting just the base command pattern (e.g., "gh pr checkout")?

The current approach defeats the purpose of intelligent whitelisting since each specific command with different arguments would need separate approval. For example:

  • gh pr checkout 1234
  • gh pr checkout 5678
  • gh pr checkout 9999

Would all be treated as different commands requiring individual whitelisting.

Could we extract the base command pattern before adding to the whitelist? The validateCommand function in webview-ui/src/utils/command-validation.ts already uses prefix matching, so we should align with that approach.

}

// Check if user selected "Add & Run" to add command to whitelist
if (response === "addAndRunButtonClicked") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates the approval logic from presentAssistantMessage.ts. Could we refactor to reuse the existing askApproval function instead?

The duplication could lead to maintenance issues if the approval flow needs to be updated in the future. Consider extracting the whitelist addition logic into a shared function that both places can call.

// Check if user selected "Add & Run" to add command to whitelist
if (response === "addAndRunButtonClicked") {
const clineProvider = await cline.providerRef.deref()
if (clineProvider) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling here. What happens if providerRef.deref() returns undefined? The code continues to line 68 and would throw an error when trying to call getState() on undefined.

Consider adding proper error handling or early return:

if (!clineProvider) {
  // Log error or handle gracefully
  return;
}

await clineProvider.postMessageToWebview({
type: "invoke",
invoke: "setChatBoxMessage",
text: `Command "${command}" added to whitelist.`,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This user-facing message should be internationalized. Consider using the t() function:

text: t('command.whitelist_added', { command }),

Also, is "whitelist" the best user-facing term here? Maybe "allowed commands" or "trusted commands" would be clearer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're duplicating the approval logic in executeCommandTool.ts, could we extract a shared function here that handles the command approval flow? This would help maintain consistency and reduce code duplication.

For example:

export async function handleCommandApproval(
  cline: Cline,
  command: string,
  options?: { allowWhitelist?: boolean }
): Promise<{ approved: boolean; addToWhitelist?: boolean }>

- Use extractCommandPattern function to extract base command patterns
- Add proper error handling for providerRef.deref()
- Use internationalization for user-facing messages
- Extract addCommandToWhitelist helper function to reduce duplication
- Add comments explaining why we can't use the shared askApproval function
- Added 'executeCommand.patternAddedToWhitelist' translation to all 17 non-English locale files
- This fixes the failing check-translations CI job
}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Catalan translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "El patró de comanda \"{{pattern}}\" s'ha afegit a la llista de comandes permeses."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper German translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "Kommandomuster \"{{pattern}}\" wurde zur Liste der erlaubten Befehle hinzugefügt."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Spanish translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "El patrón de comando \"{{pattern}}\" ha sido añadido a la lista de comandos permitidos."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper French translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "Le modèle de commande \"{{pattern}}\" a été ajouté à la liste des commandes autorisées."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Hindi translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "कमांड पैटर्न \"{{pattern}}\" को अनुमत कमांड सूची में जोड़ दिया गया है।"

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Russian translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "Шаблон команды \"{{pattern}}\" был добавлен в список разрешённых команд."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Turkish translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "Komut deseni \"{{pattern}}\" izin verilen komutlar listesine eklendi."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Vietnamese translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "Mẫu lệnh \"{{pattern}}\" đã được thêm vào danh sách lệnh được phép."

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Simplified Chinese translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "命令模式 \"{{pattern}}\" 已被添加到允许的命令列表中。"

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

}
},
"executeCommand": {
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new translation key 'executeCommand.patternAddedToWhitelist' remains in English. Please provide a proper Traditional Chinese translation.

Suggested change
"patternAddedToWhitelist": "Command pattern \"{{pattern}}\" has been added to the allowed commands list."
"patternAddedToWhitelist": "指令模式 \"{{pattern}}\" 已加入允許的指令清單。"

This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.

…action

- Updated shell-quote from v1.8.2 to v1.8.3 in webview-ui
- Added shell-quote v1.8.3 to backend dependencies
- Updated shared extract-command-pattern module to use shell-quote for proper parsing
- Added @types/shell-quote for TypeScript support
- Ensures consistent command pattern extraction across frontend and backend
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jul 1, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files. UI/UX UI/UX related or focused

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Add 'Add & Run' button to command approval UI for streamlined whitelisting

3 participants